Migration script fixes #179
Conversation
piersy
force-pushed
the
piersy/migration-script-fixes
branch
from
77fc0a2
to
f24dfe4
Compare
This looks fine to me, the rest I leave for @alecps . Out of curiosity, the gas limit of zero would be problematic because it would be used in the next block as well or is there another reason? |
|
@palango its because gas limit zero is used to determine whether a block is pre or post gingerbread and so with a gas limit of zero the trasition block wouldn't be decoded properly. It would be missing all the post gingerbread fields and all the extra fields from op-geth. I should probably add a comment to that effect. |
That makes sense. Yeah, please add a comment and we should also add a page to the spec mentioning all those details. |
|
Also, the base branch needs to be changed to |
There was a problem hiding this comment.
@piersy This looks great! Thanks for putting this together. I agree the code is much cleaner just passing in numAncients and getting rid of lastMigratedAncientBlock 👌
My only comment is that it might be helpful to keep the keepNonAncients flag bc we want to try having the rsync command run during the pre-migration (without any transformations applied). To be honest though, it's easy enough to add back so feel free to merge as is. Thanks again!
The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients.
piersy
force-pushed
the
piersy/migration-script-fixes
branch
from
f24dfe4
to
74b8139
Compare
|
@palango Added a ticket here for adding to the spec - https://github.com/celo-org/celo-blockchain-planning/issues/356 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## celo7 #179 +/- ##
===========================================
+ Coverage 61.32% 75.47% +14.15%
===========================================
Files 20 16 -4
Lines 1753 1358 -395
Branches 71 12 -59
===========================================
- Hits 1075 1025 -50
+ Misses 646 301 -345
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. |
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
* Fixed migration for datadirs without ancients The script was assuming that ancients would have been migrated and was considering the numAncients-1 to be the next block to migrate but when numAncients is zero that's a problem. Also remved logic for picking up where db migration left of for the level db since it was complicating the logic and that process takes a few seconds, which is nothing compared with the minutes taken to migrate the ancients. * Ensure that we set gas limit if migrating at pre-gingerbread point
These fixes ensure that the migration script works when there are no
ancients and also when the final celo L1 block is a pre-gingerbread block.
The migration script was assuming that ancients would be present and was
considering the numAncients-1 to be first non ancient block to migrate but when
numAncients is zero that's a problem.
Also removed logic for picking up where db migration left of for the
level db since it was complicating the logic for correctly calculating the
non ancient block to start migrating from.
Finally these changes ensure that the migration block has a non zero gas limit,
even if the parent block had a zero gas limit